Skip to content

core/txpool/blobpool: upgrade blobpool #32146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

core/txpool/blobpool: upgrade blobpool #32146

wants to merge 8 commits into from

Conversation

mask-pp
Copy link
Contributor

@mask-pp mask-pp commented Jul 4, 2025

Due to the large size of blob transactions, the goal of this PR is to reduce unnecessary repeated operations of extracting, storing, and retrieving blob transactions. As a result, only the transaction metadata is stored in limbo.
In the event of a reorganization, transactions will not be deleted from blobpool.store. Instead, their metadata will be transferred to limbo storage. When the finalization occurs, blobpool.store will be triggered to delete these transactions.

@mask-pp mask-pp requested a review from rjl493456442 as a code owner July 4, 2025 02:24
@cskiraly
Copy link
Contributor

cskiraly commented Aug 5, 2025

@mask-pp , was there a specific reason you made everything in blobTxMeta public?

@mask-pp
Copy link
Contributor Author

mask-pp commented Aug 6, 2025 •
edited
Loading

@mask-pp , was there a specific reason you made everything in blobTxMeta public?

This PR aims to avoid deleting from BlobPool.store and then storing to limbo.store when reorg appears. If the reorg appears, the tx content still stored in BlobPool.store and just needs to change the relate metadata in limbo.
But currently, I found this PR can can't work due to billy.Database cell is static, if not this PR can works well.

@mask-pp
Copy link
Contributor Author

mask-pp commented Aug 6, 2025

@mask-pp , was there a specific reason you made everything in blobTxMeta public?

This PR aims to avoid deleting from BlobPool.store and then storing to limbo.store when reorg appears. If the reorg appears, the tx content still stored in BlobPool.store and just needs to change the relate metadata in limbo. But currently, I found this PR can can't work due to billy.Database cell is static, if not this PR can works well.

Maybe we change the size of limbo.store, but it's a break change. I'm not sure whether it's worth doing or not.

@cskiraly
Copy link
Contributor

cskiraly commented Aug 7, 2025

The data flow in the original code is not ideal for larger throughput, I agree.

My question is however specific to your change of making everything public in blobTxMeta, which is like 90% of the lines changed by the PR. Is that necessary for some reason?

@mask-pp
Copy link
Contributor Author

mask-pp commented Aug 9, 2025

The data flow in the original code is not ideal for larger throughput, I agree.

My question is however specific to your change of making everything public in blobTxMeta, which is like 90% of the lines changed by the PR. Is that necessary for some reason?

Oh sorry, this PR is not mature. change blobTxMeta wants to reuse the struct so that the object can be encoded and stored in the database.
I need to know how billy.Database works at first, and then update this PR.

@mask-pp mask-pp marked this pull request as draft August 9, 2025 11:46
@mask-pp mask-pp mentioned this pull request Aug 10, 2025
Open
@mask-pp
Copy link
Contributor Author

mask-pp commented Aug 10, 2025 •
edited
Loading

Hi friends @cskiraly and @rjl493456442, sorry to bother you, the PR is combined into avoid_restore_blob_tx. Plz review again, tks.

@mask-pp mask-pp closed this Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjl493456442 rjl493456442 Awaiting requested review from rjl493456442 rjl493456442 is a code owner

Assignees

@cskiraly cskiraly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mask-pp @cskiraly